Skip to content

Refine JSON implementation for mutability, type safety, and performance - #248

Merged
AlexNachbaur merged 28 commits into
masterfrom
OKTA-998746-mutableJSON
Oct 6, 2025
Merged

Refine JSON implementation for mutability, type safety, and performance#248
AlexNachbaur merged 28 commits into
masterfrom
OKTA-998746-mutableJSON

Conversation

@AlexNachbaur

Copy link
Copy Markdown
Contributor
  • Previous implementation wasn't mutable, didn't persist its underlying value, and had a clunky developer experience
  • Working with WebAuthn, IDX, and JWT generation necessitates JSON mutability
  • Relocates JSON and concurrency utility types to separate targets, to streamline maintainability and testing

Copilot AI review requested due to automatic review settings August 20, 2025 01:29

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR significantly refines the JSON implementation to improve mutability, type safety, and performance. The changes introduce a new mutable JSON system with proper copy-on-write semantics, relocate utility types to separate targets, and streamline the overall codebase architecture.

  • Introduces a new JSON module with full mutability support and proper copy-on-write patterns
  • Moves common utility types (CopyOnWrite, ExpressionUtilities, etc.) to a separate CommonSupport target
  • Updates all JSON-related types to use the new JSON implementation instead of the previous immutable approach

Reviewed Changes

Copilot reviewed 107 out of 112 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
Sources/JSON/ Complete new JSON module implementation with mutable values and type safety
Sources/CommonSupport/ New target containing utility types previously in AuthFoundation
Tests/JSONTests/ Comprehensive test suite for the new JSON implementation
Various AuthFoundation files Updated to use new JSON types and CommonSupport utilities
Various test files Updated imports and test code to work with new JSON API
Files not reviewed (1)
  • OktaClient.xcworkspace/contents.xcworkspacedata: Language not supported

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread Tests/CommonSupportTests/CoalescedResultTests.swift
Comment thread Sources/JSON/JSON+Storage.swift
Comment thread Sources/JSON/JSON+Value.swift
Comment thread Sources/AuthFoundation/Token Management/Token+Metadata.swift Outdated
Comment thread Sources/AuthFoundation/Responses/UserInfo.swift
FeiChen-okta
FeiChen-okta previously approved these changes Aug 28, 2025
@AlexNachbaur
AlexNachbaur changed the base branch from master to feature-dpop September 17, 2025 21:25
@AlexNachbaur
AlexNachbaur force-pushed the OKTA-998746-mutableJSON branch from d5c2e41 to e6725d9 Compare September 18, 2025 01:48
@AlexNachbaur
AlexNachbaur force-pushed the OKTA-998746-mutableJSON branch from e6725d9 to 4a811b4 Compare September 24, 2025 23:32
@AlexNachbaur
AlexNachbaur changed the base branch from feature-dpop to relocate-json-libs September 25, 2025 01:18

@IldarAbdullin-okta IldarAbdullin-okta left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Base automatically changed from relocate-json-libs to master October 3, 2025 15:42
@AlexNachbaur
AlexNachbaur dismissed IldarAbdullin-okta’s stale review October 3, 2025 15:42

The base branch was changed.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 92 out of 92 changed files in this pull request and generated 5 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread Tests/OAuth2AuthTests/SessionTokenFlowTests.swift Outdated
Comment thread Sources/BrowserSignin/Providers/AuthenticationServicesProvider.swift Outdated
Comment thread Sources/AuthFoundation/Utilities/JSONDecodable.swift
Comment thread Sources/AuthFoundation/Network/APIClient.swift
Comment thread Tests/AuthFoundationTests/CredentialRefreshTests.swift

@IldarAbdullin-okta IldarAbdullin-okta left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@AlexNachbaur
AlexNachbaur merged commit 1e61e55 into master Oct 6, 2025
22 checks passed
@AlexNachbaur
AlexNachbaur deleted the OKTA-998746-mutableJSON branch October 6, 2025 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants